feat: Add support for Permissioned Keys#328
feat: Add support for Permissioned Keys#328konichuvak wants to merge 6 commits intodydxprotocol:mainfrom
Conversation
- bumps v4-proto and grpcio to get protobufs for authenticators - adds client methods for adding / removing / retrieving authenticators - adds TxOption parameter to place / cancel methods to allow signing orders on behalf of another wallet - modifies transaction building flow to include TxOptions
pnowosie
left a comment
There was a problem hiding this comment.
@konichuvak Thank you very much for you contribution. 🙌
I'd like to kindly ask you to check the comments below and respond if you think they're make sense.
Also I spotted a formatting issues. Please make sure you run poetry run black . to reformat the code before pushing.
Thanks 🙏
There was a problem hiding this comment.
I was not able to run this example. Here is the error message:
grpc._channel._InactiveRpcError: <_InactiveRpcError of RPC that terminated with:
status = StatusCode.NOT_FOUND
details = "account dydx18sukah44zfkjndlhcdmhkjnarl2sevhwf894vh not found"
Account dydx18sukah44zfkjndlhcdmhkjnarl2sevhwf894vh doesn't exists on testnet.
Have you succeeded running the example?
There was a problem hiding this comment.
Additionally, I would like to see the output when running the example. Could you introduce some print statements? e.g. examples/batch_cancel_example.py
There was a problem hiding this comment.
I am able to run the example with my own accounts on the mainnet.
The example there is just a copy of the JS client example, which does not run for me either btw.
I can parametrize the example such that the user would have to supply their own mainnet keys. Let me know if you have better ideas.
There was a problem hiding this comment.
I was able to fix the example and run it on testnet.
The reason it might work for you on mainnet is because you used owner's wallet to sign so no authenticator was needed
v4-clients/v4-client-py-v2/examples/permissioned_keys_example.py
Lines 76 to 78 in 735cb30
There was a problem hiding this comment.
glad you made it work in the end!
good, point! that should be a trader_wallet there
| return base64.b64decode(value) | ||
|
|
||
|
|
||
| def decode_authenticator(config: str, auth_type: str) -> Union[SubAuthenticator, Authenticator]: |
There was a problem hiding this comment.
Is this function used anywhere?
I'd prefer not to introduce redundant code.
There was a problem hiding this comment.
This is very helpful when you want to validate what types of authenticators are added to the account, since it handles nested structure of the sub-authenticators as well performs decoding of the base64 encoded values.
|
@konichuvak Please ignore my above comments. |
- runs linter - replaces TypedDict with dataclass - replaces starred imports with explicit ones
No worries, hope you find it useful to have as a reference in your implementation. |
I find it very useful, I will make sure to mention your contribution in my PR |
I only fixed and cleaned previous PR dydxprotocol#328 by konichuvak Author: pnowosie <pawel@nethermind.io>, konichuvak <konichuvak@proton.me>
I only fixed and cleaned previous PR dydxprotocol#328 by konichuvak Author: pnowosie <pawel@nethermind.io>, konichuvak <konichuvak@proton.me>
This is cleaned & fixed version of #328 Big thanks to the original contributor: @konichuvak --------- Co-authored-by: konichuvak <konichuvak@proton.me>
Adds functionality to enable signing transactions using the authenticator flow on the protocol.
Basically following the footsteps of #317 to add the same functionality to the Python client.